Add support for Linux cgrpoup v2, issue-4885 #5068

nezdali · 2024-03-26T16:45:59Z

Introduced ILinuxUtilizationParser interface to be able to choose between parsers during DI
Introduced new class for cgroup v2 specifics LinuxUtilizationParserCgroupV2
GetHostCpuUsageInNanoseconds() parses /sys/fs/cgroup/cpu.stat file and calculates cpu usage in microseconds
TryGetCpuUnitsFromCgroups() parses cpu quota and period from a single file /sys/fs/cgroup/cpu.max
TryGetCgroupRequestCpu() parses /sys/fs/cgroup/cpu.weight and calculates from cpu weight to shares (1024 share == 1 CPU core) in order to get the Pod's cpu request limit.
Introduced new method GetMemoryUsageInBytesFromSlices() to calculate memory stats from slices (cgroup v2 specific)
Introduced an overload in SystemResources struct where new metric is added guaranteedPodCpuUnits, which get cpu request limit (as per k8s terminology)

Microsoft Reviewers: Open in CodeFlow

… lines

mobratil · 2024-03-27T16:34:57Z

@nezdali , I deployed your change into my own cluster that has cgroupsv2 and printed SystemResources object.

The cluster have following resources in yaml:
          resources:
            requests:
              cpu: "300m"
              memory: "1.5Gi"
            limits:
              cpu: "900m"
              memory: "3.5Gi"

The printed values are following.

  System Resources: 
          Requests:
                  CPU = 309m,			(based on GuaranteedPodCpuUnits)
                  Memory = 3.5Gi,		(based on GuaranteedMemoryInBytes)
          Limits:
                  CPU = 900m,			(based on MaximumCpuUnits)
                  Memory: 15.61508560180664Gi	(based on MaximumMemoryInBytes)

While the requests for CPU corresponds to YAML specification, the memory seems to be not accurate.
Memory limit (MaximumMemoryInBytes) seems to be showing the capacity of the host machine instead of the pod limit.
The memory request (GuaranteedMemoryInBytes) contains the pod limit.

nezdali · 2024-03-28T16:45:40Z

After more research it turns out that GuaranteedMemoryInBytes is the Resource Memory Limit (in k8s terms) which is read from /sys/fs/cgroup/memory.max
And currently there is no way to get the Resource Memory Request. It is always 0 in /sys/fs/cgroup/memory.min , for cgroupv2 container, VM (AzureLinux, Ubuntu)
K8s documentation on how memory.min populated https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/2570-memory-qos/#readme

...raries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Linux/LinuxUtilizationProvider.cs

…ing/Linux/LinuxUtilizationProvider.cs Co-authored-by: Igor Velikorossov <RussKie@users.noreply.github.com>

…to issue-4885

…uhostcount in some cases

…to issue-4885

…Bytes

src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/SystemResources.cs

...raries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Linux/LinuxUtilizationProvider.cs

src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Linux/IFileSystem.cs

...braries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Linux/ILinuxUtilizationParser.cs

xakep139 · 2024-04-15T10:00:30Z

...ibraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Linux/LinuxUtilizationParser.cs

@@ -14,8 +14,10 @@ namespace Microsoft.Extensions.Diagnostics.ResourceMonitoring.Linux;
 /// This class is not thread safe.
 /// When the same instance is called by multiple threads it may return corrupted data.
 /// </remarks>
-internal sealed class LinuxUtilizationParser
+internal sealed class LinuxUtilizationParser : ILinuxUtilizationParser


Should this class be renamed to LinuxUtilizationParserCgroupV1 then?

Actually both parsers do not parse only cgroup fs at /sys/fs/cgroup , but also /proc , but the main purpose is to parse the cgroup so we rename if that makes code more readable

…ing/Linux/IFileSystem.cs Co-authored-by: Nikita Balabaev <xakep139@users.noreply.github.com>

…minor fixes

…to issue-4885

RussKie · 2024-04-16T12:08:16Z

src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Linux/IFileSystem.cs

+    /// Get directory names on the filesystem based on the provided pattern.
+    /// </summary>
+    /// <returns>string.</returns>
+    string[] GetDirectoryNames(string directory, string pattern);


Even though this is an internal class I'd caution against using mutable arrays as return values. This could lead to some gnarly hard to track bugs.
Could this be changed to something like IList<string> or IEnumerable<string>?

Makes sense, will do the changes in another PR since this one is merged already.

Add support for Linux cgrpoup v2, issue-4885

87261f3

dotnet-policy-service bot assigned nezdali Mar 26, 2024

vitalid added 4 commits March 27, 2024 11:37

Removed unnecessary using, ordered using, removed blank lines

f32c9ad

Changed path for /sys/fs/cgroup/cpu/cpu.shares

da44f5f

Experimental attribute for GuaranteedPodCpuUnits added, removed blank…

3f75b40

… lines

Exprerimental attributes added

87e64b8

Removed the experimental parameter, agreed to reuse cpuGuaranteedRequest

cda49b5

RussKie reviewed Mar 29, 2024

View reviewed changes

...raries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Linux/LinuxUtilizationProvider.cs Outdated Show resolved Hide resolved

vitalid and others added 20 commits April 4, 2024 17:06

Acceptance test to support cgroupv2

a09bfc8

Removed unnessecary using

0bc23ba

Update src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitor…

e80f7fb

…ing/Linux/LinuxUtilizationProvider.cs Co-authored-by: Igor Velikorossov <RussKie@users.noreply.github.com>

Added Exists method for HardcodedValueFileSystem

de0ca6b

Merge branch 'issue-4885' of https://github.com/nezdali/extensions in…

0a4d70e

…to issue-4885

Adjusted the Preprocessor symbol

89ee8fe

Moved cgroup version detection to a separate class

c124d5a

Summary added for ResourceMonitoringLinuxCgroupVersion class

cca3d60

Made cgroup detection class internal

1f9c963

Additional tests to cover cgroupv2 parser

742d0a4

New method to get directory names with pattern and multiple test fixes

c188df2

GetDirectoryNames method regex implementation

af86bc6

refactored GetDirectoryNames for OSFIleSystem

9ac1be5

Added test to OSFileSystemTests.cs for GetDirectoryNames

5baf6ba

Fix in OSFileSystemTests.cs

d950ad3

Excluded from code coverage logic that identifies version of Cgroups

91b0a4b

Removed unnecessary 'using', fix for GetCgroupRequestCpu to return cp…

d24f868

…uhostcount in some cases

Merge branch 'issue-4885' of https://github.com/nezdali/extensions in…

881f54b

…to issue-4885

Analyzer error fix

e4c69ae

Test fix

46cc5bf

Martin Obratil and others added 5 commits April 12, 2024 14:38

Removed one test

507f2ed

updated config path to fixtures to make osfilesystem test pass

4f6eb8b

Added ExcludeFromCodeCoverage for GetCgroupType based on nested setting

05a0994

Additional test for GetMemoryUsageInBytesFromSlices

5484960

Tests for GetCgroupCpuUsageInNanoseconds and for GetAvailableMemoryIn…

00f46f5

…Bytes

nezdali marked this pull request as ready for review April 12, 2024 20:55

mobratil requested changes Apr 15, 2024

View reviewed changes

src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/SystemResources.cs Outdated Show resolved Hide resolved

...raries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Linux/LinuxUtilizationProvider.cs Outdated Show resolved Hide resolved

dotnet-policy-service bot added the waiting-author-feedback 📭 The author of this issue needs to respond in order for us to continue investigating this issue. label Apr 15, 2024

xakep139 reviewed Apr 15, 2024

View reviewed changes

src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Linux/IFileSystem.cs Outdated Show resolved Hide resolved

xakep139 reviewed Apr 15, 2024

View reviewed changes

...braries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Linux/ILinuxUtilizationParser.cs Show resolved Hide resolved

xakep139 reviewed Apr 15, 2024

View reviewed changes

Update src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitor…

d099770

…ing/Linux/IFileSystem.cs Co-authored-by: Nikita Balabaev <xakep139@users.noreply.github.com>

dotnet-policy-service bot removed the waiting-author-feedback 📭 The author of this issue needs to respond in order for us to continue investigating this issue. label Apr 15, 2024

vitalid added 2 commits April 16, 2024 13:26

Renamed LinuxUtilizationParser to LinuxUtilizationParserCgroupV1 and …

031caa7

…minor fixes

Merge branch 'issue-4885' of https://github.com/nezdali/extensions in…

d191d28

…to issue-4885

mobratil approved these changes Apr 16, 2024

View reviewed changes

nezdali closed this Apr 16, 2024

mobratil reopened this Apr 16, 2024

mobratil enabled auto-merge (squash) April 16, 2024 12:00

RussKie reviewed Apr 16, 2024

View reviewed changes

mobratil merged commit 39b6a31 into dotnet:main Apr 16, 2024
6 checks passed

mobratil mentioned this pull request May 15, 2024

Lack of cgroups v2 Support in Microsoft.Extensions.Diagnostics.ResourceMonitoring #4885

Closed

github-actions bot locked and limited conversation to collaborators May 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Linux cgrpoup v2, issue-4885 #5068

Add support for Linux cgrpoup v2, issue-4885 #5068

nezdali commented Mar 26, 2024 •

edited by RussKie

Loading

mobratil commented Mar 27, 2024 •

edited

Loading

nezdali commented Mar 28, 2024

xakep139 Apr 15, 2024

nezdali Apr 15, 2024 •

edited

Loading

RussKie Apr 16, 2024

nezdali Apr 16, 2024 •

edited

Loading

Add support for Linux cgrpoup v2, issue-4885 #5068

Add support for Linux cgrpoup v2, issue-4885 #5068

Conversation

nezdali commented Mar 26, 2024 • edited by RussKie Loading

Microsoft Reviewers: Open in CodeFlow

mobratil commented Mar 27, 2024 • edited Loading

nezdali commented Mar 28, 2024

xakep139 Apr 15, 2024

Choose a reason for hiding this comment

nezdali Apr 15, 2024 • edited Loading

Choose a reason for hiding this comment

RussKie Apr 16, 2024

Choose a reason for hiding this comment

nezdali Apr 16, 2024 • edited Loading

Choose a reason for hiding this comment

nezdali commented Mar 26, 2024 •

edited by RussKie

Loading

mobratil commented Mar 27, 2024 •

edited

Loading

nezdali Apr 15, 2024 •

edited

Loading

nezdali Apr 16, 2024 •

edited

Loading